From c0127391bf789d61c4cf6c546e1500930e6cff2e Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 10 Sep 2014 07:26:42 -0700 Subject: [PATCH] Don't recompile git deps so frequently The previous logic for recompiling any dependency was almost entirely based on the mtimes of the relevant input files. This isn't quite what's desired for git and registry dependencies because the mtime could be fluctuating while the files aren't changing. For example: 1. Project A checks out git repo C at revision C1 2. Project A builds, records mtimes of C 3. Project B checks out git repo C at revision C2 4. Project B builds, records new mtimes of C 5. Project A is rebuilt, rebuilding C b/c mtimes are different In step 5 here C should not be rebuilt because the revision didn't actually change. This commit alters git/registry dependencies to completely bypass the --dep-info emitted and only rely on the git/registry source to inform what the fingerprint is. This is the revision/version, respectively, and should be all that's necessary to track changes to the repo and trigger a rebuild. --- src/cargo/ops/cargo_rustc/fingerprint.rs | 21 ++++++-- tests/test_cargo_compile_git_deps.rs | 64 ++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 4 deletions(-) diff --git a/src/cargo/ops/cargo_rustc/fingerprint.rs b/src/cargo/ops/cargo_rustc/fingerprint.rs index aebb93ff6..228dae234 100644 --- a/src/cargo/ops/cargo_rustc/fingerprint.rs +++ b/src/cargo/ops/cargo_rustc/fingerprint.rs @@ -2,7 +2,7 @@ use std::hash::{Hash, Hasher}; use std::hash::sip::SipHasher; use std::io::{fs, File, UserRWX, BufferedReader}; -use core::{Package, Target}; +use core::{Package, Target, PathKind}; use util; use util::hex::short_hash; use util::{CargoResult, Fresh, Dirty, Freshness, internal, Require, profile}; @@ -47,18 +47,31 @@ pub fn prepare_target(cx: &mut Context, pkg: &Package, target: &Target, let filename = filename(target); let old_loc = old.join(filename.as_slice()); let new_loc = new.join(filename.as_slice()); - let doc = target.get_profile().is_doc(); + + // We want to use the package fingerprint if we're either a doc target or a + // path source. If we're a git/registry source, then the mtime of files may + // fluctuate, but they won't change so long as the source itself remains + // constant (which is the responsibility of the source) + let use_pkg = { + let doc = target.get_profile().is_doc(); + let path = match pkg.get_summary().get_source_id().kind { + PathKind => true, + _ => false, + }; + doc || !path + }; debug!("fingerprint at: {}", new_loc.display()); // First bit of the freshness calculation, whether the dep-info file // indicates that the target is fresh. let (old_dep_info, new_dep_info) = dep_info_loc(cx, pkg, target, kind); - let are_files_fresh = doc || try!(calculate_target_fresh(pkg, &old_dep_info)); + let are_files_fresh = use_pkg || + try!(calculate_target_fresh(pkg, &old_dep_info)); // Second bit of the freshness calculation, whether rustc itself and the // target are fresh. - let rustc_fingerprint = if doc { + let rustc_fingerprint = if use_pkg { mk_fingerprint(cx, &(target, try!(calculate_pkg_fingerprint(cx, pkg)))) } else { mk_fingerprint(cx, target) diff --git a/tests/test_cargo_compile_git_deps.rs b/tests/test_cargo_compile_git_deps.rs index 90e66ca7a..91fb2e1a7 100644 --- a/tests/test_cargo_compile_git_deps.rs +++ b/tests/test_cargo_compile_git_deps.rs @@ -1080,3 +1080,67 @@ test!(git_name_not_always_needed { {compiling} foo v0.5.0 ({url}) ", updating = UPDATING, compiling = COMPILING, url = p.url(), bar = p2.url()))); }) + +test!(git_repo_changing_no_rebuild { + let bar = git_repo("bar", |project| { + project.file("Cargo.toml", r#" + [package] + name = "bar" + version = "0.5.0" + authors = ["wycats@example.com"] + "#) + .file("src/lib.rs", "pub fn bar() -> int { 1 }") + }).assert(); + + // Lock p1 to the first rev in the git repo + let p1 = project("p1") + .file("Cargo.toml", format!(r#" + [project] + name = "p1" + version = "0.5.0" + authors = [] + build = 'true' + [dependencies.bar] + git = '{}' + "#, bar.url()).as_slice()) + .file("src/main.rs", "fn main() {}"); + p1.build(); + p1.root().move_into_the_past().assert(); + assert_that(p1.process(cargo_dir().join("cargo")).arg("build"), + execs().with_stdout(format!("\ +{updating} git repository `{bar}` +{compiling} bar v0.5.0 ({bar}#[..]) +{compiling} p1 v0.5.0 ({url}) +", updating = UPDATING, compiling = COMPILING, url = p1.url(), bar = bar.url()))); + + // Make a commit to lock p2 to a different rev + File::create(&bar.root().join("src/lib.rs")).write_str(r#" + pub fn bar() -> int { 2 } + "#).assert(); + bar.process("git").args(["add", "."]).exec_with_output().assert(); + bar.process("git").args(["commit", "-m", "test"]).exec_with_output() + .assert(); + + // Lock p2 to the second rev + let p2 = project("p2") + .file("Cargo.toml", format!(r#" + [project] + name = "p2" + version = "0.5.0" + authors = [] + [dependencies.bar] + git = '{}' + "#, bar.url()).as_slice()) + .file("src/main.rs", "fn main() {}"); + assert_that(p2.cargo_process("build"), + execs().with_stdout(format!("\ +{updating} git repository `{bar}` +{compiling} bar v0.5.0 ({bar}#[..]) +{compiling} p2 v0.5.0 ({url}) +", updating = UPDATING, compiling = COMPILING, url = p2.url(), bar = bar.url()))); + + // And now for the real test! Make sure that p1 doesn't get rebuilt + // even though the git repo has changed. + assert_that(p1.process(cargo_dir().join("cargo")).arg("build"), + execs().with_stdout("")); +}) -- 2.30.2